Skip to content

ASoC: SOF: refine sof for multi-channels support - #39

Closed
RanderWang wants to merge 1 commit into
thesofproject:topic/sof-devfrom
RanderWang:volume
Closed

ASoC: SOF: refine sof for multi-channels support#39
RanderWang wants to merge 1 commit into
thesofproject:topic/sof-devfrom
RanderWang:volume

Conversation

@RanderWang

@RanderWang RanderWang commented Jul 19, 2018

Copy link
Copy Markdown

User mode lib calls info function in kcontrol to get the channel number of
volume mixer. The default info function is snd_soc_info_volsw which only
supports max two channels. Just like get/put function, info function is
bound to a bespoke info function in SOF and return correct channel number
to user.

The snd_sof_volume_info is based on snd_soc_info_volsw, and refines the
channel setting. Using standard SND_SOC_TPLG_CTL_VOLSW to register our
kcontrol function because only it supports binding all kcontrol functions to bespoke
ones. And this change doesn't affect the volume mixer which is not created by
topology and doesn't affect the topology released

Signed-off-by: Rander Wang rander.wang@linux.intel.com

@lgirdwood lgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor questions.

Comment thread sound/soc/sof/control.c Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cant do this string checking, best to have separate info() calls for volume and switch

@RanderWang RanderWang Jul 20, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I will refine it. And I also checked the Asoc, the info function would be bound only for volume case

Comment thread sound/soc/sof/topology.c Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this info be appended to SOF_TPLG_KCTL_VOL_ID ?

@RanderWang RanderWang Jul 20, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I would do this. I did this according to id, the smallest is in the head

@plbossart plbossart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please clarify the commit message. Your use of the word 'hook' isn't clear and there are missing words/grammar issues making the message difficult to understand. Thanks.

@RanderWang
RanderWang force-pushed the volume branch 2 times, most recently from 9c3f222 to ebb71bd Compare July 20, 2018 05:47
@RanderWang

Copy link
Copy Markdown
Author

Update my comments and patch

Comment thread sound/soc/sof/topology.c Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a separate fix in it's own PR.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I will make another PR

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rebase and repush, I don't see why this change is still visible.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I will do it

Comment thread sound/soc/sof/topology.c Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not following why we need to override a standard kcontrol ID here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also don't get what this is about?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart plbossart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there are still pending opens on this PR, please rebase/push and answer to questions in the comments.

Comment thread sound/soc/sof/topology.c Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rebase and repush, I don't see why this change is still visible.

Comment thread sound/soc/sof/topology.c Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also don't get what this is about?

User mode lib calls info function in kcontrol to get the channel number of
volume mixer. The default info function is snd_soc_info_volsw which only
supports max two channels. Just like get/put function,  info function is
bound to a bespoke info function in SOF and return correct channel number
to user.

The snd_sof_volume_info is based on snd_soc_info_volsw, and refines the
channel setting. Using standard SND_SOC_TPLG_CTL_VOLSW to register our
kcontrol function because only it supports binding all kcontrol functions
to bespoke ones. And this change doesn't affect the volume mixer which is
not created by topology and doesn't affect the topology released

Signed-off-by: Rander Wang <rander.wang@linux.intel.com>
@RanderWang

RanderWang commented Jul 25, 2018

Copy link
Copy Markdown
Author

Refine my patch. The detail of this change is discussing in thesofproject/soft#34
for short, I add SND_SOC_TPLG_CTL_VOLSW to bind get & put & info function to some bespoke ones. The info function can't be bound to a bespoke one with SOF_TPLG_KCTL_VOL_ID.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants